Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ zd,zv: Value derive improvements for enums #540

Merged
merged 3 commits into from
Feb 14, 2025
Merged

✨ zd,zv: Value derive improvements for enums #540

merged 3 commits into from
Feb 14, 2025

Conversation

katyo
Copy link
Contributor

@katyo katyo commented Jan 6, 2024

  • Support enums without explicit values
  • Support string representation for enums

@katyo
Copy link
Contributor Author

katyo commented Jan 6, 2024

As a future improvement: what about using external crate for ident case conversion?

zeenix
zeenix requested changes Jan 6, 2024
View reviewed changes
Copy link
Contributor

@zeenix zeenix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! Some nits to fix. Talking of which, could you kindly add emoji prefixes as per our contribution guidelines?

@zeenix
Copy link
Contributor

zeenix commented Jan 6, 2024

As a future improvement: what about using external crate for ident case conversion?

I'm not sure what you mean but I'd like to avoid adding a dep unless there is a compelling rationale presented. :)

@zeenix
Copy link
Contributor

zeenix commented Feb 7, 2024

@katyo Hi there. Do you think you'll have time to finish this soon? No hurry, just a polite ping. :)

@katyo
Copy link
Contributor Author

katyo commented Feb 19, 2024

@zeenix I'm a little bit busy lately, but I try fix this pr asap.

@katyo katyo force-pushed the derive-value-enum branch 4 times, most recently from 57bb1ce to 69ca851 Compare February 25, 2024 10:05
@katyo katyo requested a review from zeenix February 25, 2024 10:06
zeenix
zeenix requested changes Feb 25, 2024
View reviewed changes
Copy link
Contributor

@zeenix zeenix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great in general. Thanks for updating. BTW, you forgot to add emojis still. ;)

@katyo katyo force-pushed the derive-value-enum branch from 69ca851 to 0f102fc Compare February 25, 2024 14:12
@zeenix
Copy link
Contributor

zeenix commented Jul 11, 2024

@zeenix I'm a little bit busy lately, but I try fix this pr asap.

Just a gentle reminder, in hope that you're not super busy and can finish this PR. 🙏

@zeenix
Copy link
Contributor

zeenix commented Jul 30, 2024

Just a gentle reminder, in hope that you're not super busy and can finish this PR. 🙏

I guess not. 😞

@zeenix
Copy link
Contributor

zeenix commented Feb 13, 2025

I rebased this branch and since it took some effort, I already added myself as the co-author of the commits. Next I will try to address the concerns/comments raised.

@zeenix zeenix changed the title Derive value improvements for enums ✨ zd,zv: Value derive improvements for enums Feb 13, 2025
@katyo @zeenix
✨ zd,zv: Value: Add support for enums without explicit values
4cdcabe 
Co-author: Zeeshan Ali Khan <zeenix@gmail.com>
@zeenix zeenix enabled auto-merge February 14, 2025 17:59
@zeenix zeenix self-requested a review February 14, 2025 18:09
@zeenix zeenix merged commit 463e4d5 into dbus2:main Feb 14, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zeenix zeenix zeenix approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@katyo @zeenix